refactor(core): drop retired execute permission mode from capability audit - #3603
Conversation
…y audit Remove the 'execute' member from CAPABILITY_AUDIT_PERMISSION_MODES and the associated Exclude<> workaround in SkillAuditRecord, since the retired mode is already folded to 'ask' at all persistence decode sites. - scheduledTaskPermissionMode: active tasks now map to 'ask' instead of the retired 'execute' (behavioural equivalent). - executableScheduledTaskCount: filters enabled non-explore tasks instead of the now-unreachable 'execute' permissionMode literal. - CLI activation-command retains the execute→ask alias for external callers per the original comment. Refs: apache#3385 Generated-by: OpenAI Codex Signed-off-by: Yunare Maia <yunare@gmail.com>
Astro-Han
left a comment
There was a problem hiding this comment.
Reviewed at exact head ec26f69bf40c6d98c0bd619ae62505c3247ad84f. No [P0]–[P3].
The change to watch here is executableScheduledTaskCount, because the predicate is rewritten rather than merely narrowed:
// before
scheduledTasks.filter((task) => task.permissionMode === 'execute')
// after
scheduledTasks.filter((task) => task.enabled && task.permissionMode !== 'explore')Those select the same set, and it's worth writing down why, since the two expressions look nothing alike. enabled is derived as task.status === 'active' (scheduledTaskToAuditRecord), and explore is returned exactly for completed and expired. So the new predicate is active && not (completed | expired), which reduces to active. Under the old code, execute was returned for everything that was neither completed/expired nor paused — also active. Same membership, no drift in the reported count.
I checked specifically whether paused tasks could slip into the new count, since they map to ask and ask !== 'explore'. They can't: paused is not active, so enabled is false and the first conjunct rejects them.
The claim in the new comment — that the retired execute mode folded to ask identically — is consistent with execute having already been removed as a permission mode at the protocol layer, so no live caller can still be distinguishing the two.
The Exclude<CapabilityAuditPermissionMode, 'execute'> on SkillAuditRecord.permissionMode becomes redundant once execute leaves the union, and dropping it is correct rather than a widening: the field's producer at :146 only ever emits 'ask' or 'explore'.
CI note: this head's workflow run sat at action_required because it comes from a fork; I approved the run so it could execute. Gate conclusions should be drawn from that run's terminal state, not from this comment.
Astro-Han
left a comment
There was a problem hiding this comment.
Reviewed at exact head 0b1d82097f175df3ada48ea830efdeff49306e3e (biome-formatting follow-up to ec26f69bf4; the delta is line-wrapping only). No [P0]–[P3].
What this pass verified beyond the earlier review:
- No remaining
executeconsumers in the audit surface.CAPABILITY_AUDIT_PERMISSION_MODES/CapabilityAuditPermissionModeare referenced only insidecapability-audit.ts. The report's UI consumers (capability-audit-strip.tsx,module-pages.tsx,skills-panel.tsx) read neitherpermissionModenorexecutableScheduledTaskCount, so nothing downstream still branches on the dropped value. - No old-data read path can crash. The audit report is derived live (
deriveCapabilityAuditReportover in-memoryScheduledTasks) and is never persisted under this enum. Persisted task templates go through the pre-existingdecodePersistedScheduledTaskfold (scheduled-task.ts), which maps a storedexecutetoaskviadecodePersistedPermissionMode(permission.ts) — untouched by this PR. - The folding claim is backed in-repo.
RETIRED_PERMISSION_MODESinpackages/core/src/permission.tsdocuments thatexecutecompiled to the same profile asask, which is exactly what the new comment asserts. executableScheduledTaskCountmembership re-derived independently. OverSCHEDULED_TASK_STATUSES = active | paused | completed | expired: old predicate (permissionMode === 'execute') selectedactiveonly; new predicate (enabled && permissionMode !== 'explore') also reduces toactive(pausedfailsenabled). Same count, andpausedcannot slip in.
Gate: test is the only path-filtered check this change can trigger (no package.json/lockfile change, no sandbox/filesystem-worker paths) and it is terminal green on this exact head.
Astro-Han
left a comment
There was a problem hiding this comment.
Reviewed on exact head 0b1d82097f175df3ada48ea830efdeff49306e3e. No findings.
Retiring a permission mode has two ways to go wrong, and both were checked rather than assumed:
Leftover consumers. 'execute' has no remaining consumer on the audit surface — the enum is referenced only within its own file, and the UI consumers read neither permissionMode nor executableScheduledTaskCount.
Old configs crashing on read. There is no such path here. The audit report is computed live from in-memory ScheduledTask values and never persisted, and persisted templates go through decodePersistedScheduledTask, which this PR does not touch and which still folds execute → ask via RETIRED_PERMISSION_MODES in permission.ts. So durable data written before this change still decodes.
The executableScheduledTaskCount predicate was also reduced independently on both sides and comes out the same: status === 'active', with paused excluded either way.
Gating: only test is triggered on this head — no package.json/lockfile changes and no sandbox paths, so audit and the Windows sandbox lane are correctly absent rather than missing. test is terminal success on the exact head.
…audit (apache#3603) * refactor(core): drop retired 'execute' permission mode from capability audit Remove the 'execute' member from CAPABILITY_AUDIT_PERMISSION_MODES and the associated Exclude<> workaround in SkillAuditRecord, since the retired mode is already folded to 'ask' at all persistence decode sites. - scheduledTaskPermissionMode: active tasks now map to 'ask' instead of the retired 'execute' (behavioural equivalent). - executableScheduledTaskCount: filters enabled non-explore tasks instead of the now-unreachable 'execute' permissionMode literal. - CLI activation-command retains the execute→ask alias for external callers per the original comment. Refs: apache#3385 Generated-by: OpenAI Codex Signed-off-by: Yunare Maia <yunare@gmail.com> * style: apply biome formatting to capability-audit (CI fix) --------- Signed-off-by: Yunare Maia <yunare@gmail.com>
Summary
Removes the retired
executepermission mode fromCAPABILITY_AUDIT_PERMISSION_MODESand simplifies the capability audit surface, completing the cleanup described in #3385.What changed
CAPABILITY_AUDIT_PERMISSION_MODES: removedexecute— now[explore, ask](matchingPERMISSION_MODESinpermission.ts, which already dropped it).SkillAuditRecord.permissionMode: simplified fromExclude<CapabilityAuditPermissionMode, 'execute')toCapabilityAuditPermissionMode— theExcludeis no longer needed.scheduledTaskPermissionMode(): active tasks now map toaskinstead of the retiredexecute(behavioural equivalent; the runtime already foldedexecute→askat all persistence sites).executableScheduledTaskCount: filtersenabled && permissionMode !== 'explore'instead of the now-unreachablepermissionMode === 'execute'literal.activation-command.ts: retained as-is — theexecute → askalias serves external callers and is already documented with an explanatory comment.What this does NOT change
The
executemode is already retired fromPERMISSION_MODES, all persistence decode sites (decodePersistedPermissionModeinpermission.ts), the protocol epoch, and the desktop/CLI UI surfaces. This PR removes the last audit-side reference.Scope note
The
useNewTaskChoiceshadow state andresolveCreateSessionInputmentioned in #3385 appear to have been removed in prior work — they no longer exist in the currentmain. The remaining cleanup was the capability audit side handled here.Testing
scheduled-task.test.ts,tool-result-record-schema.test.ts, andtool-result-preview.test.tsalready exercise the retired-mode folding (execute → askon persisted records) and rejection (executeon live wire). This refactor does not change that behaviour.typecheckandtestwill validate; I was unable to runnpm installlocally (blocked by@xterm/xtermregistry fetch failure on this host) so I did not run the full test suite. This is disclosed transparently.AI disclosure
OpenAI Codex assisted with codebase analysis and this implementation. I reviewed the diff and take responsibility for the contribution.
Signed-off-by: Yunare Maia yunare@gmail.com